Clear cache on reload #11673
Clear cache on reload #11673
Conversation
...rc/main/java/org/enso/interpreter/node/expression/builtin/runtime/ReleaseReferencesNode.java
Outdated
Show resolved
Hide resolved
…into wip/gmt/11485-clear-cache
JaroslavTulach
force-pushed
the
wip/gmt/11485-clear-cache
branch
from
5b563d4 to
c493ec0
Compare
JaroslavTulach
force-pushed
the
wip/gmt/11485-clear-cache
branch
from
1fb2517 to
ef15d90
Compare
…into wip/gmt/11485-clear-cache
| get self = self.mr.with .get | ||
|
|
||
| clear self = | ||
| self.mr.with (ref-> ref.put Nothing) |
There was a problem hiding this comment.
Given the most recent changes in the implementation this code shouldn't set the ref to Nothing, but to a DataflowError - however I admit, I have no way to do it! I don't seem to be able to send Error into ref.put and I even managed to crash the engine while doing that #11774
There was a problem hiding this comment.
I am using a special value for the test-only implementation.
There was a problem hiding this comment.
I believe you need to (also) check for isException(). Otherwise OK from my perspective.
|
|
||
| get self = self.mr.with .get | ||
|
|
||
| clear self = |
There was a problem hiding this comment.
- if Promote broken values instead of ignoring them #11777 gets implemented
- then the
clearmethod would change its behavior - if the
refinret.put Nothingwould be aDataflowErrorthen - the result of whole
self.m.withwould be aDataflowError - and the method would terminate immediately returning the
DataflowError
distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Managed_Resource.enso
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Network/Reload_Detector.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Network/Reload_Detector.enso
Show resolved
Hide resolved
| HTTP.fetch base_url_with_slash+'test_download?length=13' | ||
| get_num_response_cache_entries . should_equal 1 | ||
| HTTP.fetch base_url_with_slash+'test_download?length=14' | ||
| HTTP.fetch base_url_with_slash+'test_download?length=15' |
There was a problem hiding this comment.
| HTTP.fetch base_url_with_slash+'test_download?length=13' | |
| get_num_response_cache_entries . should_equal 1 | |
| HTTP.fetch base_url_with_slash+'test_download?length=14' | |
| HTTP.fetch base_url_with_slash+'test_download?length=15' | |
| HTTP.fetch base_url_with_slash+'test_download?length=10' | |
| get_num_response_cache_entries . should_equal 1 | |
| HTTP.fetch base_url_with_slash+'test_download?length=14' | |
| HTTP.fetch base_url_with_slash+'test_download?length=15' |
What if we fetch a thing that was cached before? IMO this could be useful to test. But not strictly necessary.
There was a problem hiding this comment.
I think it looks good. @GregoryTravis have you tested the latest build in practice?
e.g. using this scenario?
Yes, just tested it. |
GregoryTravis
added
the
CI: Ready to merge
(cherry picked from commit 9e00b9d)
This uses #11577. It creates a
Managed_Resourcecontaining a reference that is cleared on reload; this change detects this, and clears the cache.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.